Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual Resource filters #264

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Conversation

MbolotSuse
Copy link
Contributor

Related to rancher/rancher#46330. Requires rancher/lasso#79.

Overview

This provides some basic virtual resource fields. There's a new virtual package that defines a few items including the common fields from the issue and the type for a chainable transform function.

@MbolotSuse MbolotSuse requested a review from moio August 14, 2024 15:52
@MbolotSuse MbolotSuse requested a review from a team as a code owner August 14, 2024 15:52
pkg/resources/virtual/common/util.go Outdated Show resolved Hide resolved
pkg/resources/virtual/common/util.go Outdated Show resolved Hide resolved
pkg/resources/virtual/types/types_test.go Outdated Show resolved Hide resolved
pkg/resources/virtual/virtual.go Outdated Show resolved Hide resolved
pkg/stores/sqlpartition/store.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few typos and what looks like a line of commented-out code that can be removed

moio
moio previously approved these changes Aug 20, 2024
Copy link
Contributor

@moio moio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Michael, thanks for the contribution here. It's just small things and little questions from my part. 👍

I tested the current state of the PR manually on a 2.10 Rancher build and I was able to sort/filter/paginate by id and metadata.state.name.

If you want to go the extra mile I'd appreciate a test in rancher/rancher#46015, but this is completely optional.

go.mod Outdated
@@ -123,3 +122,5 @@ require (
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace github.com/rancher/lasso => github.com/mbolotsuse/lasso v0.0.0-20240813171623-1292d08e0849
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder: please remove and re-run go mod tidy before merging.

pkg/resources/common/formatter.go Outdated Show resolved Hide resolved
pkg/stores/sqlpartition/store.go Outdated Show resolved Hide resolved
pkg/resources/virtual/common/common.go Outdated Show resolved Hide resolved
pkg/resources/virtual/types/types.go Outdated Show resolved Hide resolved
pkg/resources/virtual/common/common.go Outdated Show resolved Hide resolved
pkg/resources/virtual/common/common.go Outdated Show resolved Hide resolved
pkg/resources/virtual/common/common.go Outdated Show resolved Hide resolved
pkg/resources/virtual/virtual.go Outdated Show resolved Hide resolved
pkg/stores/sqlproxy/proxy_store.go Show resolved Hide resolved
moio
moio previously approved these changes Aug 28, 2024
Adds logic which adds virtual fields resources. This allows these fields
to be sorted/filtered on when the SQL cache is enabled. Id and
metadata.state.name were added as the first two fields.
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. Noting that there's no util_test.go but the functions in util are all tested in common_test.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants